Redistribute ECR policies and clean up policy files - #343
Open
alinabuzachis wants to merge 2 commits into
Open
Conversation
Move ECR actions and resources from paas.yaml into storage-services.yaml to free up space in paas.yaml for future sagemaker/bedrock growth. Also wildcard lightsail:Get* to reduce character count. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Fix memorydb:DeleleSnapshot typo -> memorydb:DeleteSnapshot - Remove redundant eks:DescribeNodegroup (covered by eks:Describe*) - Remove duplicate glue:Get* from application-services (already in data-services) - Remove duplicate dms endpoint resource ARN in data-services - Consolidate ecs:*CapacityProvider + ecs:PutClusterCapacityProviders into ecs:*CapacityProvider* - Consolidate cloudfront:CreateStreamingDistribution* wildcard - Wildcard bedrock:Invoke* (InvokeAgent + InvokeModel) - Sort all actions alphabetically within each SID block across all policy files Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
tremble
approved these changes
Aug 21, 2026
tremble
left a comment
tremble
left a comment
Contributor
There was a problem hiding this comment.
As a follow-up I think we need to improve the documentation around these policies.
- Where do specific policies live (some services could live in multiple files)
- When is it "ok" to use wildcards (read only; does it need to be resource limited?)
Pointing Claude at it, also raised an interesting point:
- should we add a linter which enforces a sort order?
| - backup-storage:MountCapsule | ||
| - ecr:CreateRepository | ||
| - ecr:Describe* | ||
| - ecr:GetAuthorizationToken |
Contributor
There was a problem hiding this comment.
I think we can move to ecr:Get*
Comment on lines
122
to
+123
| - bedrock:GetFoundationModel | ||
| - bedrock:ListFoundationModels |
Contributor
There was a problem hiding this comment.
I think we can move this to
bedrock:Get*
bedrock:List*
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
paas.yamltostorage-services.yaml— natural fit alongside S3, EFS, Backup, and MemoryDB, and frees up space inpaas.yamlfor futureSageMaker/Bedrock growth
memorydb:DeleleSnapshot→memorydb:DeleteSnapshot(silent permission bug — delete-snapshot was never actually granted)glue:Get*was in bothapplication-services.yamlanddata-services.yaml; duplicatearn:aws:dms:...endpoint:*resource ARN indata-services.yamllightsail:Get*,bedrock:Invoke*,cloudfront:CreateStreamingDistribution*,ecs:*CapacityProvider*eks:DescribeNodegroupwas already covered byeks:Describe*Policy size impact (JSON chars, limit: 6,144)
Sizes calculated using the same method as
deploy-test-policy.yml:lookup('template', ...) | from_yaml | to_json | length.Motivation
PR #342 proposes adding 2 new policy files (
paas-sagemaker.yaml,security-services-slr.yaml), which would bring the total to 10 — the AWS default limit for managed policies per role. This redistribution avoids that by freeing up ~900 chars inpaas.yamlthrough moving ECR tostorage-services.yaml, keeping the policy count at 8 and leaving room for future growth.Test plan
make test_policy STAGE=dev